Skip to content

ASoC: Intel: boards: tgl_sdw_rt5682: Add machine driver - #1885

Closed
naveen-manohar wants to merge 1 commit into
thesofproject:topic/sof-devfrom
naveen-manohar:tgl_rt5682-sdw
Closed

ASoC: Intel: boards: tgl_sdw_rt5682: Add machine driver#1885
naveen-manohar wants to merge 1 commit into
thesofproject:topic/sof-devfrom
naveen-manohar:tgl_rt5682-sdw

Conversation

@naveen-manohar

@naveen-manohar naveen-manohar commented Mar 11, 2020

Copy link
Copy Markdown

TGL machine driver with RT5682 connected SNDW Link0 with HDMI Audio
& DMIC Support

Signed-off-by: Naveen Manohar naveen.m@intel.com

@naveen-manohar
ASoC: Intel: boards: tgl_sdw_rt5682: Add machine driver
cd7795b 
TGL machine driver with RT5682 connected SNDW Link0 with HDMI Audio
& DMIC Support

Signed-off-by: Naveen Manohar <naveen.m@intel.com>
@naveen-manohar

naveen-manohar commented Mar 11, 2020

Copy link
Copy Markdown
Author

Patch for Debug Purposes Only

@naveen-manohar naveen-manohar mentioned this pull request Mar 11, 2020
Closed
@kv2019i

kv2019i commented Mar 11, 2020

Copy link
Copy Markdown
Collaborator

@naveen-manohar wrote:

Patch for Debug Purposes Only

Ack, I added "Draft-for-testing-not-ready-for-review" to inform people to not review this. Please remove the tag if you want to get reviews.

@plbossart

plbossart commented Mar 11, 2020

Copy link
Copy Markdown
Member

@naveen-manohar could we look into reusing the same driver as for other SoundWire platforms?
The SOF driver will provide information telling the sdw_rt711_rt1308_rt715.c driver that there is an rt5682 on link1, so in theory all you need to do is provide a descriptor for that platform, and all the rest comes for free.

@RanderWang @bardliao now that I think of it, could we support the rt700 with the same machine driver, instead of maintaining sdw_rt700.c. That way we have a single machine driver for all SOF/SDW platforms.

@RanderWang

RanderWang commented Mar 12, 2020

Copy link
Copy Markdown

@naveen-manohar could we look into reusing the same driver as for other SoundWire platforms?
The SOF driver will provide information telling the sdw_rt711_rt1308_rt715.c driver that there is an rt5682 on link1, so in theory all you need to do is provide a descriptor for that platform, and all the rest comes for free.

@RanderWang @bardliao now that I think of it, could we support the rt700 with the same machine driver, instead of maintaining sdw_rt700.c. That way we have a single machine driver for all SOF/SDW platforms.

yes, the sdw_rt711_rt1308_rt715 can support rt700 and 5682 or other sdw codecs with codec info added.

lyakh
lyakh requested changes Mar 12, 2020
View reviewed changes
Comment thread sound/soc/intel/boards/Kconfig

config SND_SOC_INTEL_TGL_SOUNDWIRE_RT5682_MACH
tristate "TGL with RT5682 in SDW mode"
depends on I2C && ACPI

@lyakh lyakh Mar 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment

Comment thread sound/soc/intel/boards/tgl_sdw_rt5682.c
ret = snd_soc_component_set_jack(component, jack, NULL);

if (ret == -EAGAIN) {
msleep(500);

@lyakh lyakh Mar 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usleep_range()

Comment thread sound/soc/intel/boards/tgl_sdw_rt5682.c

if (ret) {
dev_err(rtd->dev, "Headset Jack call-back failed: %d\n", ret);
return ret;

@lyakh lyakh Mar 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just do

if (ret)
	dev_err(...);

return ret;

@xiulipan

xiulipan commented Mar 12, 2020

Copy link
Copy Markdown

@naveen-manohar May I ask what would be the soc-acpi-intel-tgl-match.c change? Can you give an example

@naveen-manohar

naveen-manohar commented Mar 14, 2020

Copy link
Copy Markdown
Author

@naveen-manohar could we look into reusing the same driver as for other SoundWire platforms?
The SOF driver will provide information telling the sdw_rt711_rt1308_rt715.c driver that there is an rt5682 on link1, so in theory all you need to do is provide a descriptor for that platform, and all the rest comes for free.

@RanderWang @bardliao now that I think of it, could we support the rt700 with the same machine driver, instead of maintaining sdw_rt700.c. That way we have a single machine driver for all SOF/SDW platforms.

@plbossart : Agreed, Let me try re-using sdw_rt711_rt1308_rt715.c for rt5682 at link0.. Its good thought to use single machine driver.

@naveen-manohar

naveen-manohar commented Mar 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Author

@naveen-manohar May I ask what would be the soc-acpi-intel-tgl-match.c change? Can you give an example

sound/soc/intel/common/soc-acpi-intel-tgl-match.c 
@@ -76,6 +76,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_tgl_sdw_machines[] = {
		.sof_fw_filename = "sof-tgl.ri",
		.sof_tplg_filename = "sof-tgl-rt711-rt1308.tplg",
	},
	{
		.link_mask = 0x1, /* this will only enable rt5682 for now */
		.drv_name = "tgl_sdw_rt5682",
		.sof_fw_filename = "sof-tgl.ri",
		.sof_tplg_filename = "sof-tgl-rt5682.tplg",
	},

@RanderWang

RanderWang commented Mar 16, 2020

Copy link
Copy Markdown

@naveen-manohar May I ask what would be the soc-acpi-intel-tgl-match.c change? Can you give an example

sound/soc/intel/common/soc-acpi-intel-tgl-match.c 
@@ -76,6 +76,12 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_tgl_sdw_machines[] = {
		.sof_fw_filename = "sof-tgl.ri",
		.sof_tplg_filename = "sof-tgl-rt711-rt1308.tplg",
	},
	{
		.link_mask = 0x1, /* this will only enable rt5682 for now */
		.drv_name = "tgl_sdw_rt5682",
		.sof_fw_filename = "sof-tgl.ri",
		.sof_tplg_filename = "sof-tgl-rt5682.tplg",
	},

@naveen-manohar we add more features in mach table, can I update your patch and create a PR for you ?

@naveen-manohar

naveen-manohar commented Mar 19, 2020

Copy link
Copy Markdown
Author

Posted following patches based on PR:1889:
69b6d9b
0c92b28

@plbossart

plbossart commented Mar 19, 2020

Copy link
Copy Markdown
Member

Replaced by #1912

@plbossart plbossart closed this Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lyakh lyakh lyakh requested changes
@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner
@kv2019i kv2019i Awaiting requested review from kv2019i kv2019i is a code owner
@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner
@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner
@ranj063 ranj063 Awaiting requested review from ranj063 ranj063 is a code owner

Assignees

No one assigned

Labels

Draft-for-testing-not-ready-for-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@naveen-manohar @kv2019i @plbossart @RanderWang @xiulipan @lyakh